feat: reserve music generation IDs before inference with idempotent submit - #143
Merged
Conversation
…ubmit Accept Story music jobs only after persisting command/task/candidate IDs. The same idempotency key and spec replay; a reused key with a different spec conflicts. Retry/new-version mint a new attempt. Existing 202 jobs and the sync compatibility route stay.
Owner
Author
|
cursor review |
PR Review — Loreframe StudioRisk: low Automated review from Findings
Changed files
CONTRIBUTING checklist
Posted by the repo PR review workflow. Re-runs on each push to the PR. |
Code healthQuality score: 49.9/100Higher is better. The score is a trend dashboard; the independent ratchet below remains the CI gate.
Change vs PR base: +0.1 points.
Markdown, JSON catalogs and tests are out of this table. Only Most complex functions
Trend vs baseline
Warnings
Ratchet passed. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 4 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for all 4 issues found in the latest run.
- ✅ Fixed: Default key blocks repeat generations
- Sin idempotency_key ahora se reserva un id único, así un POST idéntico arranca una generación nueva en vez de reutilizar el spec_hash.
- ✅ Fixed: Replay skips worker when job missing
- Si el replay no encuentra checkpoint MiniMax, se persiste el job y se arranca el worker en lugar de devolver solo el cascarón queued.
- ✅ Fixed: Reserved count defaults disagree
- spec_snapshot ahora usa el mismo default de count=2 que el endpoint de jobs cuando el cliente omite el campo.
- ✅ Fixed: Retry keys are not replayable
- retry y new_version añaden un sufijo estable {key}:{intent} sin UUID, así un 202 perdido o un doble clic reutilizan el mismo intento.
Or push these changes by commenting:
@cursor push a2053af6a5
Preview (a2053af6a5)
diff --git a/app/_launch_runtime.py b/app/_launch_runtime.py
--- a/app/_launch_runtime.py
+++ b/app/_launch_runtime.py
@@ -2000,7 +2000,7 @@
return {"deleted": deleted, "skipped_linked": skipped_linked, "model_type": model_type}
-# ── Model pre-download ──────────────────────────────────────────────────
+# ── Model pre-download ─────────────────────────────────────────────────���
# Backs the click-to-download icon in Settings → System → Enabled Models.
# Fetches everything a generation would need (transformer + second-stage +
# modules + shared assets + text encoder) without occupying the GPU, so
@@ -3393,7 +3393,7 @@
return _load_lora_manifest()
-# ── CivitAI Browser ───────────────────────────────────────────────────
+# ── CivitAI Browser ────────────────────────────────────────────────���──
CIVITAI_BASE_URL = "https://civitai.com/api/v1"
CIVITAI_IMAGE_CDN = "https://imagecache.civitai.com/xG1nkqKTMzGDvpLrqFT7WA"
@@ -31789,8 +31789,6 @@
public = _public_minimax_music_job(existing)
public["replay"] = True
return public
- from services.music_submission import public_music_job
- return public_music_job(reserved)
job_id = str(reserved.get("job_id") or f"minimax-music-{uuid.uuid4().hex[:12]}")
task_id = str(reserved.get("task_id") or f"task-minimax-music-{job_id}")
now = time.time()
@@ -31858,6 +31856,11 @@
"idempotencyKey": reserved.get("idempotency_key"),
}
with _minimax_music_jobs_lock:
+ existing = _minimax_music_jobs.get(job_id)
+ if existing:
+ public = _public_minimax_music_job(existing)
+ public["replay"] = True
+ return public
_minimax_music_jobs[job_id] = job
_persist_minimax_music_job(job)
_publish_minimax_music_job(job)
@@ -31867,7 +31870,10 @@
name=f"minimax-music-{job_id[-6:]}",
daemon=True,
).start()
- return _public_minimax_music_job(job)
+ public = _public_minimax_music_job(job)
+ if reserved.get("replay"):
+ public["replay"] = True
+ return public
@api.get("/api/v1/stories/music-candidates/jobs/{job_id}")
diff --git a/app/services/music_submission.py b/app/services/music_submission.py
--- a/app/services/music_submission.py
+++ b/app/services/music_submission.py
@@ -78,7 +78,7 @@
provenance = request.get("provenance") if isinstance(request.get("provenance"), Mapping) else {}
model = _clean(request.get("model")) or "music-3.0"
try:
- count = max(1, min(3, int(request.get("count") or 1)))
+ count = max(1, min(3, int(request.get("count") or 2)))
except (TypeError, ValueError, OverflowError) as exc:
raise MusicSubmissionError(
"MiniMax Music candidate count must be an integer from 1 to 3",
@@ -239,9 +239,10 @@
digest = spec_hash(spec)
key = _clean(request.get("idempotency_key") or request.get("idempotencyKey"))
if not key:
- key = digest
+ # HTTP idempotency is opt-in: a missing key is a new attempt.
+ key = _token_id("idem")
if intent in {"retry", "new_version"}:
- key = f"{key}:{intent}:{uuid.uuid4().hex[:8]}"
+ key = f"{key}:{intent}"
store = MusicSubmissionStore(workspace_dir)
existing = store.load(key)
if existing:
diff --git a/tests/test_minimax_music_jobs.py b/tests/test_minimax_music_jobs.py
--- a/tests/test_minimax_music_jobs.py
+++ b/tests/test_minimax_music_jobs.py
@@ -174,6 +174,53 @@
]
+def test_replay_without_checkpoint_starts_the_worker(tmp_path):
+ namespace = _base_namespace(tmp_path)
+ reserved_job_id = "minimax-music-aaaaaaaaaaaa"
+ published = []
+ namespace.update({
+ "HTTPException": _HTTPException,
+ "_get_active_workspace": lambda: "default",
+ "_safe_join": lambda root, name: os.path.join(root, name),
+ "_persist_minimax_music_job": lambda job: published.append(("persist", job["jobId"])),
+ "_publish_minimax_music_job": lambda job: published.append(("publish", job["jobId"])),
+ "_run_minimax_music_job": lambda _job_id: None,
+ "_public_minimax_music_job": lambda job: {
+ key: value for key, value in job.items()
+ if key not in {"request", "_cancel_requested"}
+ },
+ "threading": SimpleNamespace(Thread=_DeferredThread),
+ "_reserve_story_music_submission": lambda _body, _workspace: {
+ "replay": True,
+ "job_id": reserved_job_id,
+ "task_id": f"task-{reserved_job_id}",
+ "generation_id": "gen-1",
+ "command_id": "cmd-1",
+ "candidate_id": "song-1",
+ "idempotency_key": "cmd-same-once",
+ },
+ "_load_minimax_music_job": lambda _job_id: None,
+ })
+ start = _load("start_story_music_candidates_job", namespace=namespace)[
+ "start_story_music_candidates_job"
+ ]
+
+ result = start({
+ "prompt": "cinematic dream pop",
+ "lyrics": "[Verse]\nAcross the night",
+ "workspace": "default",
+ })
+
+ assert result["jobId"] == reserved_job_id
+ assert result["status"] == "queued"
+ assert result["replay"] is True
+ assert result["jobId"] in namespace["_minimax_music_jobs"]
+ assert published == [
+ ("persist", reserved_job_id),
+ ("publish", reserved_job_id),
+ ]
+
+
def test_start_rejects_a_non_numeric_candidate_count_as_bad_input(tmp_path):
namespace = _base_namespace(tmp_path)
namespace.update({
diff --git a/tests/test_music_submission.py b/tests/test_music_submission.py
--- a/tests/test_music_submission.py
+++ b/tests/test_music_submission.py
@@ -99,8 +99,26 @@
assert retry["generation_id"] != first["generation_id"]
assert retry["intent"] == "retry"
assert retry["parent_generation_id"] == first["generation_id"]
+ replay = submit_music_generation(
+ workspace_dir=str(tmp_path),
+ request=_request(retry=True, parent_generation_id=first["generation_id"]),
+ )
+ assert replay["replay"] is True
+ assert replay["job_id"] == retry["job_id"]
+ assert replay["generation_id"] == retry["generation_id"]
+def test_missing_idempotency_key_starts_a_new_generation(tmp_path: Path):
+ payload = _request()
+ payload.pop("idempotency_key")
+ first = submit_music_generation(workspace_dir=str(tmp_path), request=payload)
+ second = submit_music_generation(workspace_dir=str(tmp_path), request=payload)
+ assert first["replay"] is False
+ assert second["replay"] is False
+ assert first["job_id"] != second["job_id"]
+ assert first["generation_id"] != second["generation_id"]
+
+
def test_missing_project_id_is_not_resolved_by_title(tmp_path: Path):
_write_library(tmp_path)
with pytest.raises(MusicSubmissionError, match="was not found"):
@@ -180,3 +198,12 @@
assert spec_hash(first) == spec_hash(second)
assert first["output_folder"] == "night-shift"
assert first["workspace_id"] is None
+
+
+def test_spec_snapshot_defaults_omitted_count_to_jobs_endpoint_default():
+ spec = spec_snapshot({
+ "prompt": "cinematic dream pop",
+ "lyrics": "[Verse]\nLa noche canta",
+ "output_folder": "night-shift",
+ })
+ assert spec["count"] == 2You can send follow-ups to the cloud agent here.
Omit a key to start a new attempt. Retry needs its own key so a lost response can replay. Replay without a MiniMax checkpoint starts the worker again. Default candidate count matches the jobs endpoint.
Owner
Author
|
cursor review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Replay can start a second worker
- El insert del job ahora es exclusivo bajo el lock y el claim rechaza un child ya running, así un replay concurrente no arranca un segundo worker ni factura MiniMax dos veces.
Or push these changes by commenting:
@cursor push 506622dbdd
Preview (506622dbdd)
diff --git a/app/_launch_runtime.py b/app/_launch_runtime.py
--- a/app/_launch_runtime.py
+++ b/app/_launch_runtime.py
@@ -2000,7 +2000,7 @@
return {"deleted": deleted, "skipped_linked": skipped_linked, "model_type": model_type}
-# ── Model pre-download ──────────────────────────────────────────────────
+# ── Model pre-download ─────────────────────────────────────────────────���
# Backs the click-to-download icon in Settings → System → Enabled Models.
# Fetches everything a generation would need (transformer + second-stage +
# modules + shared assets + text encoder) without occupying the GPU, so
@@ -3393,7 +3393,7 @@
return _load_lora_manifest()
-# ── CivitAI Browser ───────────────────────────────────────────────────
+# ── CivitAI Browser ────────────────────────────────────────────────���──
CIVITAI_BASE_URL = "https://civitai.com/api/v1"
CIVITAI_IMAGE_CDN = "https://imagecache.civitai.com/xG1nkqKTMzGDvpLrqFT7WA"
@@ -31465,8 +31465,10 @@
or str(job.get("status") or "") in _MINIMAX_MUSIC_TERMINAL
):
return None
+ child = children[child_index]
+ if str(child.get("status") or "") not in {"queued", "waiting_resource"}:
+ return None
now = time.time()
- child = children[child_index]
child.update(
status="running",
phase="requesting",
@@ -31858,6 +31860,12 @@
"idempotencyKey": reserved.get("idempotency_key"),
}
with _minimax_music_jobs_lock:
+ live = _minimax_music_jobs.get(job_id)
+ if live is not None:
+ public = _public_minimax_music_job(live)
+ if reserved.get("replay"):
+ public["replay"] = True
+ return public
_minimax_music_jobs[job_id] = job
_persist_minimax_music_job(job)
_publish_minimax_music_job(job)
diff --git a/tests/test_minimax_music_jobs.py b/tests/test_minimax_music_jobs.py
--- a/tests/test_minimax_music_jobs.py
+++ b/tests/test_minimax_music_jobs.py
@@ -218,6 +218,52 @@
assert result["status"] == "queued"
+def test_replay_without_checkpoint_starts_only_one_worker(tmp_path):
+ namespace = _base_namespace(tmp_path)
+ started = []
+
+ class CaptureThread:
+ def __init__(self, *, target, args, **_kwargs):
+ self.args = args
+
+ def start(self):
+ started.append(self.args)
+
+ namespace.update({
+ "HTTPException": _HTTPException,
+ "_get_active_workspace": lambda: "default",
+ "_safe_join": lambda root, name: os.path.join(root, name),
+ "_persist_minimax_music_job": lambda _job: None,
+ "_publish_minimax_music_job": lambda _job: None,
+ "_run_minimax_music_job": lambda _job_id: None,
+ "_public_minimax_music_job": lambda job: {
+ key: value for key, value in job.items()
+ if key not in {"request", "_cancel_requested"}
+ },
+ "_load_minimax_music_job": lambda _job_id: None,
+ "_reserve_story_music_submission": lambda _body, _workspace: {
+ "replay": True,
+ "job_id": "minimax-music-abc123def456",
+ "task_id": "task-minimax-music-abc123def456",
+ "generation_id": "gen-1",
+ },
+ "threading": SimpleNamespace(Thread=CaptureThread),
+ })
+ start = _load("start_story_music_candidates_job", namespace=namespace)[
+ "start_story_music_candidates_job"
+ ]
+ body = {
+ "prompt": "cinematic dream pop",
+ "lyrics": "[Verse]\nAcross the night",
+ "workspace": "default",
+ }
+ first = start(body)
+ second = start(body)
+ assert started == [("minimax-music-abc123def456",)]
+ assert first["jobId"] == second["jobId"] == "minimax-music-abc123def456"
+ assert second.get("replay") is True
+
+
def test_start_rejects_a_non_numeric_candidate_count_as_bad_input(tmp_path):
namespace = _base_namespace(tmp_path)
namespace.update({
@@ -363,3 +409,28 @@
namespace["_minimax_music_jobs"][job_id]["children"][0]["status"]
== "cancelled"
)
+
+
+def test_claim_refuses_an_already_running_child(tmp_path):
+ namespace = _base_namespace(tmp_path)
+ _load(
+ "_minimax_music_claim_candidate",
+ namespace=namespace,
+ )
+ job_id = "minimax-music-runningchild01"
+ job = _job(job_id, 1)
+ job.update(status="running", phase="requesting")
+ job["children"][0].update(
+ status="running",
+ phase="requesting",
+ startedAt=time.time(),
+ acquired_resources=["remote:https://api.minimax.io"],
+ )
+ namespace["_minimax_music_jobs"][job_id] = job
+
+ claimed = namespace["_minimax_music_claim_candidate"](
+ job_id, 0, "remote:https://api.minimax.io",
+ )
+
+ assert claimed is None
+ assert namespace["_minimax_music_jobs"][job_id]["children"][0]["status"] == "running"You can send follow-ups to the cloud agent here.
Insert the in-memory job under the existing lock; if the reserved job_id is already live, return that snapshot instead of starting a second provider thread.
Owner
Author
|
cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit a09ea53. Configure here.
10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
POST /api/v1/stories/music-candidates/jobsalready returns 202, but it minted IDs in memory, started the worker immediately, and did not dedupe. A lost HTTP response or a double click could start two inferences. Story destinations were not checked by ID.Final behavior
app/services/music_submission.pyreservescommand_id,generation_id,task_id,job_idandcandidate_idbefore the MiniMax thread starts.retry/new_versionmint a new attempt (parent_generation_idon retry).library_revisionis CAS.generationId,commandId,candidateId,idempotencyKey,replay.POST /api/v1/stories/music-candidates(sync) is unchanged.Scope
app/services/music_submission.py(new)_launch_runtime.py(+36 lines)docs/development/MUSIC_SUBMISSION.md+fase4.mdNot in this PR: local ACE/Music3 still uses
generateMusic; server-side attach of the finished song (phase 5); router extract (phase 9).Tests
pytest tests/test_music_submission.py tests/test_minimax_music_jobs.py— 14 passedbash scripts/validate_local.sh— OK (E2E 7/7)8b010a68Risks / limits
Base
origin/main8b010a68(merge #142). Do not auto-merge. Phase 5/6/8 wait for this merge.Note
Medium Risk
Touches the critical
_launch_runtime.pymusic job path and introduces durable idempotency/Story validation; behavior changes for duplicate submits and lost HTTP responses, though scope is narrow and well-tested without GPU.Overview
Adds
music_submissionso story music jobs are accepted durably before inference: command/generation/task/job/candidate IDs, TaskRegistry rows, and on-disk idempotency records are created up front. Same idempotency key + spec replays the same IDs; mismatched spec returns 409. Story targets are validated by project/cue/candidate ID (optional library_revision CAS), not title.POST /api/v1/stories/music-candidates/jobsnow calls reservation first instead of minting IDs only in memory. Replays return an existing live job when present; if the reservation exists but the in-memory job is gone, it restarts the worker with the reserved IDs. Concurrent replays that miss the checkpoint only start one worker. The 202 body gains additive fields (generationId,commandId,candidateId,idempotencyKey,replay).Contract is documented in
MUSIC_SUBMISSION.md;fase4.mdmarks phase 4 tasks done. Tests cover submission dedup, Story validation, post-reserve worker failure, and launch replay/concurrency behavior.Reviewed by Cursor Bugbot for commit a09ea53. Configure here.